feat(execd): extract exit_code for command error output#523
feat(execd): extract exit_code for command error output#523Pangjiping wants to merge 1 commit intoalibaba:mainfrom
exit_code for command error output#523Conversation
Pangjiping
force-pushed
the
feat/run_command_exit_code
branch
from
526b62f to
6cb66ac
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 526b62fad2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // ExitCode is the exit code of the command | ||
| ExitCode int `json:"exit_code"` |
There was a problem hiding this comment.
Because ErrorOutput is reused for non-command failures and ServerStreamEvent.ToJSON marshals it directly (components/execd/pkg/web/model/codeinterpreting.go:99-101), making ExitCode a plain int means every error that does not explicitly set it will now serialize as "exit_code": 0. That affects ordinary interpreter/SQL failures such as ContextCancelled (components/execd/pkg/runtime/jupyter.go:121-124) and DBInitError (components/execd/pkg/runtime/sql.go:45-47), so any client that starts trusting the new field will misread “no exit code available” as a successful process exit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, add an unified exit status code 255 for these non-command execution failures, to avoid the ambiguity caused by exit_code=0.
Pangjiping
force-pushed
the
feat/run_command_exit_code
branch
from
6cb66ac to
e359efb
Compare
3 participants
Summary
exit_codefor command error outputTesting
Breaking Changes
Checklist